Conversation
meithecatte
left a comment
meithecatte
left a comment
There was a problem hiding this comment.
For now I'm not looking at the details of how the zerocopy API works. I'll take a closer look later.
| T::Numeric: Immutable, | ||
| T::Numeric: FromZeros, | ||
| { | ||
| fn only_derive_is_allowed_to_implement_this_trait() {} |
There was a problem hiding this comment.
I think this could use a comment linking to google/zerocopy#287, so that this doesn't scream "crimes! damn crimes!" at anyone who looks at this later.
| { | ||
| (my_candidate.read_unaligned::<zerocopy::pointer::BecauseImmutable>() ^ T::ALL_BITS) | ||
| == T::EMPTY | ||
| } |
There was a problem hiding this comment.
This bit manipulation looks suspicious. Please add tests that exercise this. Or, maybe something like BitFlags::from_bits_truncate(x).bits() == x would be better, in terms of code reuse.
zebreus
force-pushed
the
zerocopy-support
branch
from
1cda797 to
b67dbea
Compare
With the zerocopy feature activated we implement some zerocopy traits containing an `only_derive_is_allowed_to_implement_this_trait` function. That is fine as long as we know what we are doing.
| #[must_use] | ||
| #[inline(always)] | ||
| pub fn validate_bits(bits: T::Numeric) -> bool { | ||
| // SAFETY: We're truncating out all the invalid bits so it will | ||
| // only be different if there are invalid bits set. | ||
| (bits & T::ALL_BITS) == bits | ||
| } |
There was a problem hiding this comment.
Honestly, I'm not sure if this makes sense as a public API.
| // Dereference the pointer to the candidate | ||
| let candidate = | ||
| my_candidate.read_unaligned::<zerocopy::pointer::BecauseImmutable>(); | ||
| return BitFlags::<T>::validate_bits(candidate); |
There was a problem hiding this comment.
| return BitFlags::<T>::validate_bits(candidate); | |
| BitFlags::<T>::validate_bits(candidate) |
| { | ||
| (my_candidate.read_unaligned::<zerocopy::pointer::BecauseImmutable>() ^ T::ALL_BITS) | ||
| == T::EMPTY | ||
| // TODO: Currently this assumes that the candidate is aligned. We actually need to check this beforehand |
There was a problem hiding this comment.
Not sure if this is something I would've caught. Do you have any links I could reference for example zerocopy implementations for similar types?
2 participants
This PR adds a flag that enables implementations for zerocopy traits on BitFlags.
Converting a
BitFlagsinto the underlying number is trivial and always safe, so we can just derive those traits. Converting from a number toBitFlagsis more tricky, because not every bit-pattern is valid. To assert that the bit-pattern is valid this PR provides a custom implementation of thezerocopy::TryFromBytestrait. Custom implementations for zerocopy traitsTryFromBytesseem somewhat discouraged but supported, so it should be fine to implement that.